Fix issue with config and state updates across instances #48
Conversation
Simplified base card HTML setup
|
Hi @raldred |
|
Hey @DanteWinters so this PR is purely to address the config problem you highlighted in your comment on #47 save for the few other little tweaks listed above. Which I ran into when testing. The main changes to keep each instance of the card isolated is the config cloning and the shadow dom to keep styles and dom queries within the individual card. This PR It's branched off your current main and does not include the things from #47 I'd be more than happy to jump on a hangout or zoom to discuss if you have time. |
lux-power-distribution-card.js
Outdated
| const inv_info_element = this.card.querySelector("#taskbar-grid"); | ||
| if (inv_info_element) { | ||
| inv_info_element.innerHTML = hf.generateStatus(this.config); | ||
| const card = document.createElement('ha-card'); |
There was a problem hiding this comment.
Apologies for the delayed feedback. I implemented this branch and I get an error here. The solutions was to change
const card = to this.card =
I'm not sure which is better for the card, to use a const at first and then assign, or directly initiate the card as part of the class. I'm reading up on the functions and implementations that you have that I don't know, once I'm done there and this part is fixed, I'll gladly merge this PR and then focus on the other one
There was a problem hiding this comment.
No problem, sorry as per my other comment, I didn't commit the change to this.card correctly.
It needs to be an instance attribute on the class so It can passed to the history graph functions.
We could return the const card, out of the createCard function, but it's completely fine to use instance attributes.
lux-power-distribution-card.js
Outdated
| const header = document.createElement("h1"); | ||
| header.classList.add('card-header'); | ||
| header.appendChild(document.createTextNode(this._config.title)); | ||
| card.appendChild(header); |
There was a problem hiding this comment.
I tried adding the title. This line also need updating in same as the comment above. Changed to this.card and now it works. Thanks for adding that feature
There was a problem hiding this comment.
Sorry about that, I noticed this when I was testing the history graph which didn't work so I adjusted things to use this.card but didn't commit it properly. My mistake, i've updated the PR.
|
Hi @raldred I'll create a new release once both PRs are sorted and tested. |
|
Thanks @DanteWinters I think it's probably best if I create a separate PR to address the small issues individually. I didn't test the parallel stuff, I will improve my mock setup so that I can properly test those areas in future against any changes. Sorry about that. |
This PR fixes the issue with config and state updates across instances, it was caused by config being shared between instances by updates being made to the hass config and base_config instead of creating a clone of the config per instance.
This also...
status_codesrequired.In fixing the config and update issues, I've removed a bunch of refreshing logic which is not required since the card will update immediately...
I have verified this with using some mock helpers entities, (screenshot below)